Skip to content

fix: role-mention only matches bot's own roles (with guild_create cache)#246

Closed
chengli wants to merge 1 commit intoopenabdev:mainfrom
chengli:feat/role-mention-filter
Closed

fix: role-mention only matches bot's own roles (with guild_create cache)#246
chengli wants to merge 1 commit intoopenabdev:mainfrom
chengli:feat/role-mention-filter

Conversation

@chengli
Copy link
Copy Markdown

@chengli chengli commented Apr 12, 2026

Summary

  • Fix role-mention detection to only match roles assigned to the receiving bot, preventing cross-triggers in multi-bot guilds
  • Cache the bot's own role IDs from guild_create events (not ready) in a RwLock<HashSet> — zero per-message API calls
  • Early-exit when msg.mention_roles is empty (skip cache read entirely)

Closes #245

Details

Bug: msg.mention_roles.iter().any(|r| msg.content.contains(...)) triggers on any role mentioned in the message, not just the bot's own roles. In a guild with bots A (role X) and B (role Y), mentioning role Y incorrectly wakes up bot A.

Fix: replace string matching with a set intersection against the bot's cached roles:

let is_role_mentioned = if !msg.mention_roles.is_empty() {
    let cached_roles = self.bot_role_ids.read().await;
    msg.mention_roles.iter().any(|r| cached_roles.contains(&r.get()))
} else {
    false
};

Technical notes:

  1. Why guild_create instead of ready? ready.guilds contains UnavailableGuild stubs. Guild member data may not be available yet, particularly for larger guilds, causing .member() lookups to 404. guild_create fires per guild after ready with complete data.

  2. Global HashSet vs per-guild HashMap: We use a flat HashSet<u64> for the cache. Discord Role IDs are globally unique and msg.mention_roles only contains roles valid for the current guild, so a flat cache is safe and keeps lookups simple.

  3. Known limitation: If a role is added to the bot while it is running, it won't be recognized until restart. A guild_member_update handler could keep the cache hot if dynamic role assignment becomes a requested feature.

Files changed

File Change
src/discord.rs Add bot_role_ids: Arc<RwLock<HashSet<u64>>> to Handler; replace string-match with cache lookup; add guild_create handler to populate cache
src/main.rs Initialize empty bot_role_ids cache

Test plan

  • cargo check passes ✅ (verified locally)
  • Single-bot guild: role mention still triggers the bot
  • Multi-bot guild: role-mentioning bot A's role does NOT trigger bot B
  • Bot has no guild roles: role mention does not trigger the bot
  • Large guild: role cache populates correctly via guild_create
  • No role mentions in message: early-exit, no cache read

fix: role-mention only matches bot's own roles (with guild_create cache)
6a933b0 
The existing role-mention check matched any role mentioned in the
message, not just roles assigned to the receiving bot. In multi-bot
guilds this caused cross-triggering: mentioning bot B's role would
also wake up bot A.

Replace string matching against msg.content with a set intersection
against the bot's own cached role IDs. The cache is populated from
guild_create events (not ready, which only has UnavailableGuild stubs
for larger guilds) and stored in a RwLock<HashSet<u64>>. Early-exit
when msg.mention_roles is empty to skip the cache read entirely.

Closes openabdev#245
@chengli chengli requested a review from thepagent as a code owner April 12, 2026 04:31
@CHC-Agent CHC-Agent mentioned this pull request Apr 12, 2026
Open
@thepagent thepagent added the p2 Medium — planned work label Apr 15, 2026
@masami-agent
Copy link
Copy Markdown
Contributor

masami-agent commented Apr 16, 2026

Review — PR #246

Good fix for a real multi-bot guild problem. The approach is correct.

✅ What looks good

  1. guild_create over ready — correct choice. ready.guilds only has UnavailableGuild stubs for larger guilds, so role data isn't available there.
  2. Arc<RwLock<HashSet<u64>>> — appropriate shared state pattern. Discord Role IDs are globally unique, so a flat set works.
  3. Fast path — mention_roles.is_empty() check avoids acquiring the read lock when there are no role mentions.
  4. Error handling — guild_create failure logs a warning but doesn't crash the bot.

🔴 Must fix before merge

1. Merge conflict with main
src/main.rs handler struct initialization has changed significantly on main (new fields: router, adapter, allow_bot_messages, trusted_bot_ids; removed: pool, reactions_config). Please rebase onto latest main.

2. CI not triggered
This is a fork PR — CI needs maintainer approval to run. Once rebased, a maintainer will approve the workflow run.

🟡 Non-blocking

3. Role cache not updated after guild_create
If the bot is assigned a new role after startup, the cache won't reflect it until next restart. Consider adding a guild_member_update listener in a follow-up PR.

Summary

Logic is sound, minimal change, solves a real problem. Rebase onto main and we're good to approve.

@masami-agent masami-agent mentioned this pull request Apr 16, 2026
Closed
chaodu-agent
chaodu-agent reviewed May 1, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@chaodu-agent chaodu-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 STALE / OBSOLETE — Bug already fixed on main by removing role-mention detection entirely.

Baseline Check (Step 0)
Field Value
State OPEN
Mergeable CONFLICTING
Created 2026-04-12 (19 days ago)
Changes +41 / -4, 2 files
Labels p2, closing-soon

Main branch status: The current main branch is_mentioned logic is:

let is_mentioned = msg.mentions_user_id(bot_id)
    || msg.content.contains(&format!("<@{}>", bot_id));

No role-mention detection at all. The cross-bot triggering bug was resolved by removing role-mention detection entirely.

Four-Question Framework

1. What problem does it solve?
In multi-bot guilds, the old is_mentioned logic checked msg.mention_roles with string matching — any role mention triggered ALL bots. PR fixes this by caching bot's own role IDs via guild_create events.

2. How does it solve it?

  • bot_role_ids: Arc<RwLock<HashSet<u64>>> on Handler
  • Populated via guild_create events (correct choice over ready)
  • Set intersection against cached roles instead of string matching
  • Early-exit when msg.mention_roles is empty

3. What was considered?
guild_create vs ready — well-reasoned. Flat HashSet vs per-guild HashMap — correctly notes Role IDs are globally unique.

4. Is it the best approach?
Technically sound for the problem it targets, but the problem no longer exists on main. Role-mention detection was removed entirely.

Traffic Light

🟢 INFO

  • The guild_create approach is correct and well-documented
  • Arc<RwLock<HashSet>> is the right concurrency pattern
  • Early-exit on empty mention_roles is a nice optimization
  • Excellent PR description

🔴 CRITICAL

  1. Bug already fixed on main — Role-mention detection was removed from is_mentioned entirely. The cross-bot triggering issue no longer exists.
  2. Handler struct completely refactored — Main's Handler now has router, adapter, allow_bot_messages, trusted_bot_ids, participated_threads, multibot_threads, bot_turns, allow_dm. The PR targets the old struct.
  3. Merge conflicts — Both files conflict with current main.

Recommendation: Close this PR. The bug is already fixed on main. If role-mention support is desired as a feature in the future, the guild_create caching approach from this PR is the right pattern and could be referenced.

Reviewed by 超渡法師 🔃 chaodu Backlog triage

@chaodu-agent
Copy link
Copy Markdown
Collaborator

chaodu-agent commented May 1, 2026

Closing — the role-mention triggering bug was already resolved on main by removing role-mention detection from is_mentioned entirely. If role-mention support is desired as a future feature, the guild_create caching approach from this PR is a good reference. Thanks for the contribution! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chaodu-agent chaodu-agent chaodu-agent left review comments

@thepagent thepagent Awaiting requested review from thepagent thepagent is a code owner

Assignees

No one assigned

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 3 days p2 Medium — planned work pending-contributor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix: role mentions trigger unintended bots in multi-bot guilds

4 participants

@chengli @masami-agent @chaodu-agent @thepagent